[CF-4208] Add --name/--status filtering to on-prem application list - #3428
[CF-4208] Add --name/--status filtering to on-prem application list#3428Paras Negi (paras-negi-flink) wants to merge 4 commits into
--name/--status filtering to on-prem application list#3428Conversation
|
🎉 All Contributor License Agreements have been signed. Ready to merge. |
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds application-list filtering support for Flink on-prem by wiring CLI flags into the CMF “filter” query parameter and updating the on-prem test server + integration fixtures to exercise name/status filtering behavior.
Changes:
- Add
--name(wildcard suffix supported) and--statusflags toflink application list, and build a CMFfilterquery from them. - Extend the CMF REST client
ListApplicationsto accept an optional filter and include it in requests. - Update the on-prem test server to emulate CMF-side filtering and add new integration test cases + golden outputs.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| test/test-server/flink_onprem_handler.go | Emulates CMF filter behavior for on-prem application listing. |
| pkg/flink/cmf_rest_client.go | Adds filter support to the REST client listing call. |
| internal/flink/command_application_list.go | Introduces --name/--status flags and composes the CMF filter query. |
| internal/flink/command_application_list_test.go | Unit-tests the filter composition helper. |
| test/flink_onprem_test.go | Adds integration scenarios for list filtering. |
| test/fixtures/output/flink/application/*.golden | Golden outputs for the new filtering scenarios and updated help text. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| switch key { | ||
| case "name": | ||
| name, _ := app.Metadata["name"].(string) | ||
| if prefix, isWildcard := strings.CutSuffix(value, "*"); isWildcard { | ||
| return strings.HasPrefix(name, prefix) | ||
| } | ||
| return name == value | ||
| case "state": | ||
| if app.Status == nil { | ||
| return false | ||
| } | ||
| jobStatus, ok := (*app.Status)["jobStatus"].(map[string]interface{}) | ||
| if !ok { | ||
| return false | ||
| } | ||
| state, _ := jobStatus["state"].(string) | ||
| return strings.EqualFold(state, value) | ||
| default: | ||
| return true | ||
| } |
| for _, expr := range strings.Split(filter, ",") { | ||
| key, value, found := strings.Cut(expr, "=") | ||
| if !found { | ||
| continue | ||
| } |
Steven Gagniere (sgagniere)
left a comment
There was a problem hiding this comment.
Hi, I have a few comments:
|
|
||
| cmd.Flags().String("environment", "", "Name of the Flink environment.") | ||
| cmd.Flags().String("name", "", `Filter the Flink applications by name. Supports wildcards, for example "my-app*".`) | ||
| cmd.Flags().String("status", "", "Filter the Flink applications by status.") |
There was a problem hiding this comment.
I think we should name this state to match the filter key.
| // "state=" filter, per the cmf-sdk-go GetApplications filter documentation. Unknown values are | ||
| // still forwarded (the server returns no matches rather than erroring); this list only drives | ||
| // the advisory --status warning. | ||
| var allowedApplicationStatuses = []string{"RUNNING", "FINISHED", "FAILED", "CANCELED", "RECONCILING", "COMPLETED", "UNKNOWN"} |
There was a problem hiding this comment.
Just to confirm: are all of these statuses accepted filter arguments? The spec's description for filterParam implies that only RUNNING or FAILED are valid; so the description may be out of date for the spec.
f5c4f57 to
7a68092
Compare
The on-prem Flink (CPF/CMF) list commands loop over a hardcoded 100-item page size with no way to control it, so listing a large environment costs many serial round trips (~51 requests for 5000 applications). Add a --page-size flag to every paginated on-prem list command so callers can fetch the full list in fewer, larger requests. It defaults to 100, which preserves today's behavior and output. Collapse the 12 duplicated pagination loops in the CMF client into a single generic listAllPages helper that takes the page size. The requested size is passed through to the CMF page/size API; the loop still terminates on the first empty page. Server-side filtering for `application list` is deliberately left out and will follow in a separate PR stacked on this one. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…logical pagination cases - Reword the --page-size help so it reads as a per-request batch size, not a cap on total results (regenerate the affected help/usage goldens). - Add an integration case for --page-size > MaxInt32 (3000000000), exercising the previously untested upper-bound branch of getPageSize (reuses the invalid golden). - Drop the statement/detached-savepoint --page-size cases: those mock handlers ignore the size param, so the cases only verified flag plumbing (already covered by the help goldens) rather than pagination. The applications case (mock genuinely pages) plus TestListAllPages remain the real coverage. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…e validation Per review on #3424: - Default the flag to 100 (matching the documented behavior) instead of the 0 sentinel, and drop the redundant "Defaults to 100." text since pflag now renders "(default 100)". - Change the flag type to Int32 and read it directly, removing the hand-rolled range validation. Out-of-range input is now rejected by the flag parser itself (no client-side validation not backed by the spec), which also closes the int32-overflow concern. Non-positive values fall back to the default page size in listAllPages. Regenerated the affected help and usage goldens. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
7a68092 to
bbb15bf
Compare
Stacked on the --page-size PR. Add server-side filtering to `flink application list` via --name (supports a "*" suffix wildcard) and --status, composed into the CMF applications "filter" query (name=<value>,state=<value>). ListApplications gains a filter parameter that is applied via the SDK's .Filter(...) before pagination. An unrecognized --status prints a [WARN] to stderr but still queries, since the CMF server treats an unknown state as a no-match rather than an error. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
bbb15bf to
a153efd
Compare
|



Release Notes
New Features
--nameand--statusfilters to the on-prem (Confluent Platform / CMF)confluent flink application listcommand, to filter large environments server-side instead of "list everything then grep".Checklist
Whatsection below whether this PR applies to Confluent Cloud, Confluent Platform, or both.Test & Reviewsection below.Blast Radiussection below.What
Confluent Platform (CMF on-prem) only — filtering half of CF-4208; Confluent Cloud
flinkcommands are untouched.flink application listhad no filtering, so at scale the only pattern was "list everything then grep" (CF-4202). This adds server-side filtering:--name— by application name; supports a trailing*wildcard (e.g.--name my-app*).--status— by Flink job state (RUNNING,FINISHED,FAILED,CANCELED,RECONCILING,COMPLETED,UNKNOWN).Both compose into CMF's single generic
filterquery asname=<value>,state=<value>;ListApplicationsgains afilterargument applied before pagination. An unknown--statusprints a[WARN]to stderr but still queries, since CMF treats an unknown state as a no-match (matchingstatement list --status).Blast Radius
application list; with neither flag set, behavior is unchanged (nofilterparam). NoCmfClientInterface/mock change. Non-breaking, easy to revert.References
--page-sizeto on-prem Flink list commands #3424.Test & Review
TestBuildApplicationFilter: name / wildcard / status / combined composition (name=a*,state=RUNNING).--nameexact + wildcard,--statusmatch / no-match / invalid (asserts[WARN]+ empty), combined--name+--status; help golden regenerated.make lintclean.🤖 Generated with Claude Code